Support the password config item type #2463
Conversation
There was a problem hiding this comment.
one comment. i think its confusing that i cannot tell as an end user when the value stored is empty. why not show it as empty if it is empty?
and one question. what happens if you tab through the inputs? is the tab considered a keystroke and does it clear the password value rather than focusing on the next input?
|
This PR has been released (on staging) and is available for download with a embedded-cluster-smoke-test-staging-app license ID. Online Installer: Airgap Installer (may take a few minutes before the airgap bundle is built): Happy debugging! |
| // Merge new values with existing ones | ||
| mergedValues := make(map[string]string) | ||
| maps.Copy(mergedValues, existingValues) | ||
| maps.Copy(mergedValues, newValues) |
There was a problem hiding this comment.
this will mutate the original. why not make a copy? https://go.dev/play/p/t4OsmJsnaDV
| if _, exists := maskedValues[item.Name]; exists { | ||
| maskedValues[item.Name] = PasswordMask | ||
| } |
There was a problem hiding this comment.
perhaps you should not set the mask here if its empty
| if _, exists := maskedValues[item.Name]; exists { | |
| maskedValues[item.Name] = PasswordMask | |
| } | |
| if val := maskedValues[item.Name]; val != "" { | |
| maskedValues[item.Name] = PasswordMask | |
| } |
| // override values from the config values store | ||
| if value, ok := storedValues[item.Name]; ok { | ||
| configValue.Value = value | ||
| if item.Type == "password" { |
There was a problem hiding this comment.
i feel like you should do the same here for sub items. even if we do not support password types, it cant hurt and we will introduce a bug in the future if we ever do.
There was a problem hiding this comment.
i see there is not type. never mind
What this PR does / why we need it:
Adds support for the password config item type
Which issue(s) this PR fixes:
SC-126349
Does this PR require a test?
Yes
Does this PR require a release note?
Does this PR require documentation?
NONE
Loom
https://www.loom.com/share/9cddba2435a14326badbe8ed57313709?sid=911515cb-68a5-4912-af3f-8ebd3338d13a